Agree that this is under specified. I am not sure that spelling this out explicitly in the RFC text works --- its not clear whether the specified rules have any loopholes.

My current belief is that the rule we want here is exactly equivalent to coherence. So the RFC should say something along to the effect of "function bodies are treated as downstream crates for the purposes of trait checking and type inference".

As this (most excellent) example by QuineDot demonstrates

#3373 (comment)

it is important to restrict both:

• which impls are allowed inside
• which inside impls are visible from the outside, for the purpose of type inference reasoning that "I know of a single impl for this trait, so _ must be that type", where "that type" might end up being a type from a function's body.

Ah, I missed this extra bit of context: rust-lang/rust#121621, where it is suggested that we want more restritive rules than coherence here specifically to avoid fixind point two from above.

@tmandry Is this something you can review (or direct to someone who can)?

I can't tell if this is changing the RFC in a meaningful way, or just clarifying it. If it is a change from before, it would probably be best to start a new RFC.

From a quick look this is in line with the original RFC, and if @joshtriplett is willing to consider it a "friendly amendment" I think it can be merged.

Made a few editorial changes, and made one comment about moving the example out of the motivation section. With that change made, LGTM; no objections.

Isn't this going against T-types solution ? (which is being implemented in rust-lang/rust#122747, @lcnr)
It was even approved by T-lang.

@Urgau Thanks for catching that. @lcnr, can you help resolve the discrepancy here?

I'd personally just keep the RFC as "we lint if the existance of the impl is observable outside of its containing item", I don't think there is much value in explaining the exact algorithm used for that in an RFC. I also thought we generally expect the reference/guidelevel explanation of RFCs to get out of date and do not bother updating them, instead referring to the reference/docs.

The idea implemented in rust-lang/rust#122747 is as follows:

Lint impls inside of bodies unless:

• the body is of an unnamed const, or
• the self type is also local to the body, or
• for trait impls: after replacing all generic params and types in the local to the body with inference variables in the "impl trait ref", there are at least 2 global impls which could also prove that trait bound

This should mean that in all cases where the local impl may apply outside of the body, we already fail with ambiguity even when ignoring the impl

@lcnr I'm deferring entirely to the types team here for whether you want to make changes/suggestions to this or close it.

It'd be nice if the rules we have are documented somewhere. Updating the RFC is one possible option; others could work as well.

whether you want to make changes/suggestions to this or close it

My only feelings is that it is often hard to tell the status and intended behaviour of in-development features. Whether or not updating RFCs is appropriate I don't know... and now we're getting off-topic.

This PR is waiting on you @rust-lang/types.

@rustbot label +T-types

It'd be nice if the rules we have are documented somewhere. Updating the RFC is one possible option; others could work as well.

We have some documentation in The RFC life-cycle, where we say that RFCs are historical snapshots of a decision made in the past and "only very minor changes should be submitted as amendments".

cc @rust-lang/types slightly in favor of closing this and keeping the RFC outdated. Don't have a strong opinion here and would be fine if one if you wants to push this over the finish line. Would close this PR in a few days unless somebody wants to take it.

I don't have fully fleshed out thoughts here and am not confident my preference is actually correct :>

my feeling is that we tend not update RFCs and RFCs arent supposed to act as an up to date reference for behaviour so I'm not sure it makes sense to land this 🤔 I would like to have a properly written description of behaviour somewhere though because I remember this lint being particularly involved. Maybe in the dev guide? Or maybe just module level docs of where we implement the lint if it's not already.

I'm not sure to what extent this is actually t-types though. When it was decided that we wouldn't do a "type system correct" handling of this lint I think this became a lot more T-compiler/T-lang as the rules are entirely arbitrary now and not really much to do with the type system.